Skip to content

What I currently have#21

Open
Gabby-Moon wants to merge 5 commits intogrc-cohort-21:mainfrom
Gabby-Moon:main
Open

What I currently have#21
Gabby-Moon wants to merge 5 commits intogrc-cohort-21:mainfrom
Gabby-Moon:main

Conversation

@Gabby-Moon
Copy link
Copy Markdown

This was as much as I could get done within the timeframe.

Copy link
Copy Markdown

@auberonedu auberonedu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you didn't get a chance to fully finish, but nice job with the tokenizer!

Comment on lines +46 to +47
char last = word.charAt(word.length()-1);
if(last != '.'){
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works well! You can also use endsWith here

Comment on lines +38 to +41
String line = scanner.nextLine();
// makes an array from the line using spaces gets rid of extra spaces with +
// source for \\s vs \\s+ : https://stackoverflow.com/a/15625711
String[] list = line.split("\\s+");
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for citing your source! You can also rely on scanner.next which handles the whitespace for you automatically

Comment on lines +22 to +29
@Test
void testTokenizeWithMultipleSpaces() {
LowercaseSentenceTokenizer tokenizer = new LowercaseSentenceTokenizer();
Scanner scanner = new Scanner("hello hi hi hi hello hello");
List<String> tokens = tokenizer.tokenize(scanner);

assertEquals(List.of("hello", "hi", "hi", "hi", "hello", "hello"), tokens);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice test!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants